-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Fix NPE in SemanticTextHighlighter #129509
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix NPE in SemanticTextHighlighter #129509
Conversation
If the model settings is null then the field is guaranteed to not have any indexed content. Closes elastic#129501
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
|
Hi @jimczi, I've created a changelog YAML for you. |
…settings' into semantic_highlighter_null_model_settings
Mikep86
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for the quick fix! I left one comment about cluster features.
...nce/src/yamlRestTest/resources/rest-api-spec/test/inference/90_semantic_text_highlighter.yml
Show resolved
Hide resolved
kderusso
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks for the quick fix
...nce/src/yamlRestTest/resources/rest-api-spec/test/inference/90_semantic_text_highlighter.yml
Show resolved
Hide resolved
Samiul-TheSoccerFan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, Is the error happening silently? I tried with both sparse and dense vector in both ES3 and 8.17.2 but could not reproduce the error. I tried something like this:
PUT _inference/text_embedding/mistral-text_embedding
{
"service": "mistral",
"service_settings": {
"model": "mistral-embed",
"dimensions": 1024,
"similarity": "dot_product",
"rate_limit": {
"requests_per_minute": 240
},
"api_key": "API_KEY"
},
"chunking_settings": {
"strategy": "sentence",
"max_chunk_size": 250,
"sentence_overlap": 1
}
}
PUT my-index
{
"mappings": {
"properties": {
"keyword_field": {
"type": "keyword"
},
"inference_field": {
"type": "semantic_text",
"inference_id": "mistral-text_embedding"
}
}
}
}
POST my-index/_search
{
"query": {
"semantic": {
"field": "inference_field",
"query": "What is Elasticsearch?"
}
},
"fields": ["_inference_fields"],
"highlight": {
"fields": {
"inference_field": {} -- tried with `type=semantic` as well
}
}
}
...nce/src/yamlRestTest/resources/rest-api-spec/test/inference/90_semantic_text_highlighter.yml
Show resolved
Hide resolved
Good question. I think it's OK if the bug hasn't been ported to Serverless? |
|
@Samiul-TheSoccerFan your reproduction query doesn't return any document. Try with this: |
💚 Backport successful
|
If the model settings is null then the field is guaranteed to not have any indexed content. Closes elastic#129501
If the model settings is null then the field is guaranteed to not have any indexed content.
Closes #129501